ci: post sticky PR comment with fallow audit findings by jrusso1020 · Pull Request #954 · heygen-com/hyperframes

jrusso1020 · 2026-05-19T01:40:49Z

Stacked on #949. Merge that first.

What

Adds a sticky PR comment from the Fallow audit job so reviewers see findings inline instead of digging through CI logs.

How

The Fallow audit job (added in #942) now runs in four steps:

Run audit + capture markdown — bunx fallow audit ... --fail-on-issues --format pr-comment-github > /tmp/fallow-comment.md with set +e. Captures the markdown output and the audit exit code. Includes a non-empty guard so transient fallow crashes don't post a blank sticky.
Post sticky comment (findings) — marocchino/sticky-pull-request-comment@v2.9.1, only when exit_code != '0'. Uses fallow's built-in  sentinel as the header so each subsequent run updates the same comment.
Remove stale sticky comment (clean run) — same action with delete: true, only when exit_code == '0'. Clears any previous "findings" sticky after a follow-up commit fixes the issue.
Fail if audit found issues — re-emits the captured exit code so the job still gates the PR on new findings.

The sticky-comment steps both have continue-on-error: true so fork PRs (which run with a read-only GITHUB_TOKEN regardless of workflow permissions:) don't fail the whole job on a comment they can't post; the audit gate still fires via step 4.

Permissions

pull-requests: write is scoped to the fallow job only — moved out of the workflow-level permissions: block (per Vance's review). Build, lint, test, smoke jobs etc. keep the workflow's read-only default. Job-level permissions override the workflow block, so this scope-down is one extra permissions: stanza, no other changes needed.

What the comment looks like

When there are findings: a collapsible markdown table listing severity, rule, file:line, and description — same content fallow normally writes to stdout.

When there are no findings: no comment is posted, and any stale "findings" sticky from a previous run is deleted.

Test plan

YAML parses
Local: bunx fallow audit --fail-on-issues --format pr-comment-github emits markdown to stdout AND exits 1 when findings exist
Job-level permissions: block correctly overrides workflow-level (verified against GitHub Actions docs)
Verify in CI: this PR adds workflow lines only — should fall through the "clean run" branch and delete any prior fallow comment (creating none, since none exists yet)
Verify on a follow-up PR that intentionally introduces a finding: comment should appear

Follow-ups

If sticky comments prove too coarse, can swap to per-line inline review comments (--format review-github) — bigger UX shift

jrusso1020 · 2026-05-19T02:08:32Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

vanceingalls

Workflow change is small, surgical, and well-commented. Three steps (run + capture, post sticky, re-fail) is the right decomposition and the sentinel-matching with fallow's  is the correct sticky pattern.

Audited: .github/workflows/ci.yml (entire file, end-to-end at 3d59ebb7); verified marocchino/sticky-pull-request-comment@52423e01 resolves to tag v2.9.1 and oven-sh/setup-bun@0c5077e5 resolves to v2; verified fallow ^2.75.0 is in root package.json so bunx fallow resolves from node_modules (PR-body claim holds).

Strengths

The "Remove stale sticky comment (clean run)" step (ci.yml:135-140) is the right call — without it, a PR that goes red then green would leave the findings comment around forever and reviewers would think the PR is still broken. Nice catch.
set +e + capturing $? to GITHUB_OUTPUT (ci.yml:121-126) is the cleanest way to keep the audit gating while still letting the sticky-post step run. Cleaner than relying on continue-on-error at the step level.
The top-of-file comment on the pull-requests: write widening (ci.yml:5-9) is exactly the kind of forward-looking note that pays off later. It names the future split point (fork PRs → separate workflow) before someone else has to figure it out.

Findings

important — ci.yml:9 (pull-requests: write at the workflow level). This grants every job in ci.yml PR-write tokens — Build, Lint, Format, Typecheck, Test, both smoke jobs, CLI smoke, filesize, semantic-pr-title. Only fallow needs it. The blast-radius concern is: any of those jobs runs bun install against user-controlled package.json (lifecycle scripts) or bun run build against user-controlled source, all under a write-scoped token. The in-diff comment acknowledges this and defers; my push is that the right shape is available now — move permissions: to the job level inside fallow: only and revert the top-level to pull-requests: read. That keeps the audit-comment poster working without expanding any other job's authority. Job-level permissions: override the workflow-level block. One-line fix, no other tradeoffs.
nit — ci.yml:127-133 posts /tmp/fallow-comment.md unconditionally on non-zero exit, but if bunx fallow crashes before producing meaningful markdown, the file may be empty or partial. Cheap guard: if [ ! -s /tmp/fallow-comment.md ]; then echo "fallow produced no output" > /tmp/fallow-comment.md; fi after the audit. Avoids posting a blank sticky on infra failure.
nit — ci.yml:124-125 would benefit from set -o pipefail in case fallow's output is ever piped (today it isn't; this is paranoia against future edits to the line).
nit — fork PRs: with the pull_request trigger (not _target), GitHub gives forks a read-only token regardless of the workflow permissions: block, so the sticky-post step will fail for fork contributors. Not a security issue (correct direction of failure), but worth a one-line graceful degradation — continue-on-error: true on the two sticky-comment steps so a fork PR doesn't fail CI on a comment it can't post. The audit gate still fires via "Fail if audit found issues."

Notes

Test-plan checkbox 3 ("Verify in CI: this PR should post a 'No findings' comment on itself") — given the "Remove stale sticky comment (clean run)" branch, a no-findings PR shouldn't post a comment; it should delete a (nonexistent) one. The verification on this PR is "no fallow comment appears." Worth nudging the description for clarity.

Verdict: APPROVE
Reasoning: The behavior change is correct and the design is sound. The workflow-level pull-requests: write is an important call-out the author already flagged for follow-up; scoping it to the fallow job in this PR would be one line and worth doing before merge, but it isn't a blocker given there's no fork trigger here.

Review by Vai

miguel-heygen

APPROVE — well-structured CI workflow.

Verified:

Exit code capture pattern (set +e → $? → GITHUB_OUTPUT) is correct
marocchino/sticky-pull-request-comment@52423e01640425a022ef5fd42c6fb5f633a02728 SHA matches v2.9.1
Sticky comment lifecycle is right: post on findings, delete on clean run — prevents stale "0 findings" comments
--format pr-comment-github outputs to file, read via path: — avoids ${{ }} injection vectors
No security concern: fallow output is from static analysis, not untrusted PR metadata

Non-blocking note: pull-requests: write is workflow-wide. The inline comment documents why this is acceptable. If fork PRs ever become a concern, moving fallow to its own workflow would scope the permission.

jrusso1020 · 2026-05-19T02:23:47Z

Thanks @vanceingalls @miguel-heygen — addressed all four points:

Fixed

🔧 Important — pull-requests: write scope: moved from workflow-level to job-level on fallow: only. The rest of `ci.yml` (Build, Lint, Format, Typecheck, Test, smoke jobs, etc.) keeps the workflow-level pull-requests: read default. Job-level permissions override workflow-level, so this is the clean one-stanza scope-down you suggested.
🔧 Nit — empty markdown guard: added if [ ! -s /tmp/fallow-comment.md ]; then echo "fallow produced no output — see the job logs above." > /tmp/fallow-comment.md; fi after the audit. No more blank stickies on transient crashes.
🔧 Nit — fork PR graceful degradation: continue-on-error: true on both sticky-comment steps. Fork PRs (read-only GITHUB_TOKEN regardless of permissions: block) won't fail the whole job on a comment they can't post; the audit gate at the end still fires.
📝 Note — test plan wording: PR body updated. Clean-run behavior is now correctly described as "no comment posted; any stale finding-sticky from a previous run is deleted."

Skipped

set -o pipefail — fallow's output isn't piped today; paranoia for a future change isn't worth the line.

Force-pushed @ 7c1098da.

Mirrors the same `fallow audit --base ... --fail-on-issues` check that runs in CI, but locally against HEAD so issues surface at commit time instead of after the push round-trip. Scoped to `packages/**` source files via the glob — non-code edits (README, docs, top-level configs) skip the hook entirely. Measured locally: ~5s in parallel with the existing lint/format/typecheck checks. Doesn't extend wall-clock time because typecheck (~11s) is the long pole, and lefthook runs commands in parallel. The default `--gate new-only` means inherited findings don't block the commit — same gate behavior as CI, so local pre-commit and PR audit agree.

After fallow's auto-fix de-exports unused symbols, oxlint surfaces them as no-unused-vars. This PR deletes those orphan declarations outright. Biggest cleanup: studio/src/icons/SystemIcons.tsx shrinks from 132 to 57 lines — 33 unused icon wrappers and their phosphor-icon imports deleted. Other deletions across 14 more files covering paired getter/setters, helper functions, dead env constants, internal components with no callers, and cascading unused imports. Cascade-causing files held back for follow-up PRs: renderOrchestrator barrel of captureCost re-exports, telemetry/portUtils/remote barrels, Button.tsx + ui/index.ts (would orphan whole file), studioMotion type re-exports. Test plan: typecheck clean across 8 packages, oxlint + oxfmt clean, fallow audit exit 0 (remaining findings inherited), cli + studio vitest suites pass.

Reviewers shouldn't have to dig through CI logs to see what fallow flagged. With this change, on every PR the fallow job posts (or updates) a sticky comment containing the full audit report formatted as a collapsible markdown table. The comment uses fallow's built-in `pr-comment-github` format, which already emits a `` sentinel. `marocchino/sticky-pull-request-comment@v2.9.1` matches that header so each run replaces the previous comment instead of stacking new ones. The job now runs in three steps: 1. Run `fallow audit ... --format pr-comment-github` with `continue-on-error: true` so the comment posts even when the audit fails. Exit code is captured. 2. Post (or update) the sticky comment with the captured output. 3. Re-emit the audit exit code so the job still fails-the-build on new findings. Bumps the workflow's `pull-requests` permission from read to write, needed for the sticky-comment poster to call the issues API.

The base branch was changed.

miguel-heygen

Re-reviewed — APPROVE.

All prior feedback addressed in the force-push:

pull-requests: write scoped to job level (not workflow-wide) — addresses the least-privilege concern
Empty markdown guard added (if [ ! -s /tmp/fallow-comment.md ]) — handles fallow crash edge case
continue-on-error: true on both sticky-comment steps — graceful degradation for fork PRs
PR description updated to match actual behavior

CI all green including fallow audit, full regression suite, typecheck, lint, build.

vanceingalls

Re-review on a67436b4. All three prior findings ADDRESSED — including the important one. Diff is clean: top-level permissions: pull-requests: read, job-level pull-requests: write scoped to fallow: only.

Findings status

important (workflow-level pull-requests: write) — ADDRESSED. .github/workflows/ci.yml:4-6 is now permissions: contents: read / pull-requests: read. ci.yml:88-90 adds job-level permissions: contents: read / pull-requests: write on fallow: only, with a comment that names exactly the right reason ("the rest of ci.yml keeps the workflow-level pull-requests: read default so build / lint / test tokens can't post or modify PR comments"). Build, Lint, Format, Typecheck, Test, smoke jobs, filesize, semantic-pr-title now run with read-only PR scope. Blast-radius pinched as recommended.
nit (empty/partial sticky markdown guard) — ADDRESSED. ci.yml:113-115: if [ ! -s /tmp/fallow-comment.md ]; then echo "fallow produced no output — see the job logs above." > /tmp/fallow-comment.md; fi. Good fallback message too — points reviewers at the log rather than dropping a blank comment.
nit (fork-PR fail-soft on sticky-comment steps) — ADDRESSED. ci.yml:121 and ci.yml:130 both have continue-on-error: true on the two sticky-comment uses-blocks. Fork PRs won't fail CI on a comment they can't post; the audit gate (Fail if audit found issues step) still runs and gates the build.

New code worth a callout

ci.yml:128-134 — "Remove stale sticky comment (clean run)" using delete: true is the right pattern; a PR that goes red→green now sheds its findings comment rather than carrying a stale one indefinitely. (Pre-existing, kept from the prior review SHA; still strong.)

CI status (verbatim, head a67436b4)

All required checks SUCCESS: Build, Lint, Format, Typecheck, Test, Test: runtime contract, CLI smoke (required), Smoke: global install, Fallow audit, File size check, regression-shards (8 shards), preview-regression, player-perf, regression, Semantic PR title, Detect changes, Preflight (lint + format), Graphite / mergeability_check. No FAILURE conclusions. mergeStateStatus: BLOCKED is review-required (this approval clears it), not CI.

baseRefName: main — no stacked-PR CI gap.

Verdict: APPROVE
Reasoning: Important finding addressed exactly as recommended (job-level scoping); both nits cleanly handled. CI fully green on main base. Ship it.

Review by Vai (re-review)

jrusso1020 force-pushed the cleanup/fallow-delete-orphans branch from 7f10c63 to 3c5961a Compare May 19, 2026 01:55

jrusso1020 force-pushed the ci/fallow-pr-comments branch from af04a20 to 3d59ebb Compare May 19, 2026 01:56

jrusso1020 mentioned this pull request May 19, 2026

ci: run fallow audit in lefthook pre-commit #948

Merged

4 tasks

jrusso1020 mentioned this pull request May 19, 2026

refactor: delete orphan declarations flagged by fallow #949

Merged

vanceingalls previously approved these changes May 19, 2026

View reviewed changes

miguel-heygen previously approved these changes May 19, 2026

View reviewed changes

jrusso1020 force-pushed the ci/fallow-pr-comments branch from 3d59ebb to 7c1098d Compare May 19, 2026 02:22

jrusso1020 force-pushed the cleanup/fallow-delete-orphans branch from 3c5961a to 0c088b7 Compare May 19, 2026 02:22

jrusso1020 force-pushed the cleanup/fallow-delete-orphans branch from 0c088b7 to 521cbd7 Compare May 19, 2026 02:32

jrusso1020 added 3 commits May 19, 2026 02:32

jrusso1020 force-pushed the ci/fallow-pr-comments branch from 7c1098d to a67436b Compare May 19, 2026 02:32

jrusso1020 force-pushed the cleanup/fallow-delete-orphans branch from 521cbd7 to e03d038 Compare May 19, 2026 02:32

Base automatically changed from cleanup/fallow-delete-orphans to main May 19, 2026 04:11

miguel-heygen approved these changes May 19, 2026

View reviewed changes

vanceingalls approved these changes May 19, 2026

View reviewed changes

jrusso1020 merged commit efc5f05 into main May 19, 2026
38 checks passed

jrusso1020 deleted the ci/fallow-pr-comments branch May 19, 2026 04:28

vanceingalls mentioned this pull request May 19, 2026

docs(lambda): document webm support + simplify-review fixes #953

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: post sticky PR comment with fallow audit findings#954

ci: post sticky PR comment with fallow audit findings#954
jrusso1020 merged 3 commits into
mainfrom
ci/fallow-pr-comments

jrusso1020 commented May 19, 2026 •

edited

Loading

Uh oh!

jrusso1020 commented May 19, 2026 •

edited

Loading

Uh oh!

vanceingalls left a comment

Uh oh!

miguel-heygen left a comment

Uh oh!

jrusso1020 commented May 19, 2026

Uh oh!

miguel-heygen left a comment

Uh oh!

vanceingalls left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jrusso1020 commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Permissions

What the comment looks like

Test plan

Follow-ups

Uh oh!

jrusso1020 commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vanceingalls left a comment

Choose a reason for hiding this comment

Uh oh!

miguel-heygen left a comment

Choose a reason for hiding this comment

Uh oh!

jrusso1020 commented May 19, 2026

Uh oh!

miguel-heygen left a comment

Choose a reason for hiding this comment

Uh oh!

vanceingalls left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jrusso1020 commented May 19, 2026 •

edited

Loading

jrusso1020 commented May 19, 2026 •

edited

Loading